Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add map trail glow #101

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add map trail glow #101

wants to merge 5 commits into from

Conversation

CoolandonRS
Copy link

Discussion Reference

All new features must be discussed prior to code review. This is to ensure that the implementation aligns with other design considerations. Please link to the Discord discussion:

https://discord.com/channels/531175899588984842/534490472224391169/1283529780494602293

Is this a breaking change?

Breaking changes require additional review prior to merging. If you answer yes, please explain what breaking changes have been made.

No

This probably has more then a few code style issues, if you point them out I can clean them up. Tried to generally copy the gist of the code style but probably messed up in more then a few spots.

@dlamkins
Copy link
Member

Okay, a couple of things that we need to think through / adjust:

  • There are too many settings for this single feature. Let's please decide on some reasonable defaults and keep only what is necessary.
    • If you think folks will want to tinker, we can use the AdvancedDefaults.cs to add the nitty gritty settings to the advanced.yaml file.
  • The glowing chase lights are only visible on sections of the trail that are translucent. By default, the trails are opaque unless there is a verticality difference. Took me a while to figure out why I couldn't see them at all. 😅

@dlamkins
Copy link
Member

What is the public Distance property for?

@CoolandonRS
Copy link
Author

CoolandonRS commented Sep 14, 2024

Ah. Didn't think that trails were opaque by default. I tried messing around with color of beads but couldn't seem to find something that worked super well. Maybe brightening/darkening the beads according to whether its closer to white or black (as to avoid the previous issue with brightening white trails being invisible)?

As for removable/hideable settings Length can definitely be removed. It's not super noticeable what it does and would probably be more confusing than anything. Speed and Count both feel useful to me, and opacity/brightness is probably useful so the bead stands out less egregiously from the rest of the trail should you want that. Perhaps opacity could be set to a fixed percentage of trail opacity, something like 150%.

Distance is the value of the total distance covered by the trail when following it directly (Like how long a drive is when following the directions instead of directly from point A to B). I made it public on the off chance it could be useful in other situations, but it's entirely subjective and can be changed.

- Remove the bead length setting, replacing it with a constant of 1.5f
- Change bead opacity to a percentage of trail opacity, and switch it to an advanced setting.
- Make the bead the inverted color of the trail
- Remove the remnants of the toggle setting that was accidentally left in
@CoolandonRS
Copy link
Author

@dlamkins are more changes needed?

@dlamkins
Copy link
Member

dlamkins commented Sep 24, 2024

@dlamkins are more changes needed?

I'll be able to take a closer look later this week. Also, is there a reason this all happens in its own draw pass instead of around the same area where the trails themselves are drawn?

@CoolandonRS
Copy link
Author

CoolandonRS commented Sep 26, 2024

Also, is there a reason this all happens in its own draw pass instead of around the same area where the trails themselves are drawn?

I'm not entirely sure what you mean. On StandardTrail.Minimap#45-47 it will render the glow bead using the same spritebatch. It doesn't just recolor the existing segment of the line because then the line couldn't be shortened to ensure all beads are of similar length. Distance/angle/opacity need to be recalculated for the same reason. Hopefully that answers your question?

Make the bead spacing consistent between long and short trails at the cost of the setting being slightly less intuitive
@dlamkins
Copy link
Member

dlamkins commented Nov 2, 2024

I will try to review this again either this weekend or next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants